Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Does this fix losing the context of the tx due to nginx internal redirects(POSTS all recorded as GETs etc.)? If so thats a good first step in the right direction.

Does this fix losing the context of the tx due to nginx internal redirects(POSTS all recorded as GETs etc.)? If so thats a good first step in the right direction.

Yes, it does. The base PR (#326) didn't do that but I could add #273 (see my initial commit) which does. Both two PR's are necessary. I also faced the duplicated tx in case of internal redirect, this is why I started to research.

Will have a look and test this out!

Hey I have done some initial testing, it appears to work as intended.

@airween For ngx_http_modsecurity_get_module_ctx part, I'd suggest to use with the original realip part, which checks r->internal || r->filter_finalize too.

When I use it with the old modsecurity, I don't know why I removed the internal and flter_finalize part.

https://github.com/nginx/nginx/blob/476d6526b2e8297025c608425f4cad07b4f65990/src/http/modules/ngx_http_realip_module.c#L544-L568

Hi @liudongmiao,

@airween For ngx_http_modsecurity_get_module_ctx part, I'd suggest to use with the original realip part, which checks r->internal || r->filter_finalize too.

When I use it with the old modsecurity, I don't know why I removed the internal and flter_finalize part.

https://github.com/nginx/nginx/blob/476d6526b2e8297025c608425f4cad07b4f65990/src/http/modules/ngx_http_realip_module.c#L544-L568

thanks for suggestion - you mean here I should replace that function with your suggestion?

Could you send a review with your solution?

@liudongmiao ping.

@airween Yes, I suggest you use original version from nginx, and check whether use nginx version or my version.

@airween However, my patched version works well since the pr I maked 2 years ago.

@airween Yes, I suggest you use original version from nginx, and check whether use nginx version or my version.

Thanks - just to clarify: you mean we should add the extra condition to check if the ctx is null or not, right? So now I copied from your solution this:

    ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
    if (ctx == NULL) {

but we should change to this:

    ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
    if (ctx == NULL && (r->internal || r->filter_finalize)) {

It would be nice to know what the r->filter_finalize does, because Nginx's documentation does not explains that (r->internal is covered in the docs).

@airween In my old cases, I just want to get context, ignore any nginx's internal status.

You should check it carefully.

@airween In my old cases, I just want to get context, ignore any nginx's internal status.

You should check it carefully.

Thanks - then I keep it as it is. We can add other conditions if everyone has an issue.

Thanks for all the help guys, merging now.